Skip to content

feat: add --fetch flag for direct benchmark suite download - #26

Merged
mintybasil merged 9 commits into
mainfrom
feat/direct-fetch
Apr 22, 2026
Merged

feat: add --fetch flag for direct benchmark suite download#26
mintybasil merged 9 commits into
mainfrom
feat/direct-fetch

Conversation

@mintybasil

Copy link
Copy Markdown
Member

Summary

  • Add --fetch <url> flag to run subcommand that downloads a .tar.zst benchmark suite, extracts it to a
  • Streaming decompression (no disk buffering of the archive — memory efficient)
  • Auto-detect single-subdirectory wrapping (handles archives that wrap content in a subdirectory)
  • Mutually exclusive with --data-dir (conflicts_with enforced by clap)
  • 3 unit tests covering flat extraction, subdirectory extraction, and bad URL rejection
  • Clippy clean, all tests pass

Closes #14

Test Plan

  • cargo test -p bento-bench — 3/3 pass
  • cargo clippy -p bento-bench -- -D warnings — zero warnings
  • Manual end-to-end: bento-bench run --fetch <real-url> --iterations 1

@zeroklaw

Copy link
Copy Markdown
Member

Fix pushed in 70f73b7.

Root cause: conflicts_with = "data-dir" panics at runtime because clap resolves argument IDs during parsing and cannot locate data_dir when it lives inside a flattened CommonArgs struct — the ID namespace doesn't cross #[clap(flatten)] boundaries at runtime validation.

Fix: Removed the conflicts_with attribute entirely. The existing code already gives --fetch unconditional priority over --data-dir in run() — when --fetch is provided, data_dir is simply not used. Both flags being specified simultaneously produces correct, unambiguous behavior (fetch wins), so no runtime validation is needed.

If mutual exclusivity error messages are desired in the future, the correct approach would be an explicit anyhow::bail! check at the top of run() comparing self.fetch.is_some() with whether --data-dir was set to a non-default value via ArgMatches.

@zeroklaw

Copy link
Copy Markdown
Member

Fix pushed in 8496a19.

Root cause: tempfile::TempDir deletes the directory when it's dropped. fetch_suite created a TempDir, copied its path into a PathBuf, then returned the PathBuf — but TempDir went out of scope at the end of the function and deleted the directory before the caller could use it. The suite was being extracted correctly, then immediately deleted.

Fix: Replace tempdir().path().to_path_buf() with tempdir().keep(). TempDir::keep() converts the handle into a plain PathBuf without registering a destructor, so the directory persists for the lifetime of the process (the OS cleans it up on exit, which is appropriate for a CLI tool).

@mintybasil
mintybasil merged commit 1401c2e into main Apr 22, 2026
2 checks passed
@mintybasil
mintybasil deleted the feat/direct-fetch branch April 22, 2026 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add direct fetch

2 participants